Skip to content

Add tests for async BaseClient#73

Merged
hexoul merged 1 commit intoline:mainfrom
hexoul:test/add-async-base
Dec 10, 2025
Merged

Add tests for async BaseClient#73
hexoul merged 1 commit intoline:mainfrom
hexoul:test/add-async-base

Conversation

@hexoul
Copy link
Collaborator

@hexoul hexoul commented Dec 7, 2025 •
edited by coderabbitai bot
Loading

Motivation:

  • Verify the behavior of the newly added async BaseClient and increase test coverage.

Modifications:

  • Added tests/test_async_base_client.py using pytest-asyncio.
  • Fixed BaseClient.request() to properly await the AsyncRetrying coroutine.
  • Implemented __aenter__ in BaseClient to support the async context manager protocol.

Result:

  • Verified async client functionality with unit tests.
  • Fixed runtime errors regarding AsyncRetrying and context manager usage.

Summary by CodeRabbit

  • New Features

    • BaseClient now supports context manager syntax for both synchronous and asynchronous usage.

  • Tests

    • Comprehensive test coverage added for asynchronous BaseClient operations, including request handling, retries, and error scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@hexoul hexoul added this to the 0.7.0 milestone Dec 7, 2025
@hexoul hexoul self-assigned this Dec 7, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 7, 2025 •
edited
Loading

Walkthrough

This PR adds asynchronous context manager support to the BaseClient class and introduces comprehensive async test coverage. Changes include implementing __aenter__ for async and __enter__ for sync context management, updating test configuration to support async tests, adding a new test module that validates async client behavior across HTTP methods, headers, retries, and error handling.

Changes

Cohort / File(s) Summary
BaseClient Context Manager Support
centraldogma/_async/base_client.py, centraldogma/_sync/base_client.py		 Adds __aenter__ method to async BaseClient returning self; adds __enter__ method to sync BaseClient returning self. Async variant also modifies request execution to await AsyncRetrying iterator instead of returning directly.
Test Configuration
pytest.ini, pyproject.toml		 Adds asyncio configuration (mode=auto, fixture_loop_scope=function) to pytest.ini; adds pytest-asyncio>=1.2.0 to dev dependencies in pyproject.toml.
Async Test Suite
tests/test_async_base_client.py		 New comprehensive test module validating async BaseClient behavior: header construction, parameter propagation, request methods (GET/POST/DELETE/PATCH/PUT/OPTIONS), retry logic, error handling, timeout and connection limit configurations using respx mocking.
Unasync Tool Configuration
utils/run-unasync.py		 Adds __aenter__ → __enter__ replacement mapping to align async context manager conversion with sync equivalent during unasync transformation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • The async retry behavior change in centraldogma/_async/base_client.py (transition from returning to awaiting AsyncRetrying iterator) requires verification of control flow correctness
  • Test coverage across multiple HTTP methods and scenarios is extensive but follows consistent patterns; validate that fixtures and mocking setup are appropriate
  • The unasync replacement mapping should be verified to correctly synchronize async and sync implementations

Poem

🐰 A client now awaits the async call,
Context managers embrace it all,
Tests cascade through every method—
GET, POST, DELETE, what's left unsaid?
With retries and headers standing tall! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary change: adding comprehensive tests for async BaseClient, which aligns with the main additions of tests/test_async_base_client.py and supporting infrastructure changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16bd29a and c766b0c.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • centraldogma/_async/base_client.py (2 hunks)
  • centraldogma/_sync/base_client.py (1 hunks)
  • pyproject.toml (1 hunks)
  • pytest.ini (1 hunks)
  • tests/test_async_base_client.py (1 hunks)
  • utils/run-unasync.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
centraldogma/_async/base_client.py (1)
centraldogma/_sync/base_client.py (1)
  • _request (88-102)
tests/test_async_base_client.py (2)
centraldogma/_async/base_client.py (3)
  • BaseClient (25-116)
  • _set_request_headers (83-86)
  • request (68-81)
centraldogma/exceptions.py (2)
  • UnauthorizedException (39-42)
  • NotFoundException (33-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: test (macos-latest, pypy3.9)
  • GitHub Check: test (windows-latest, pypy3.9)
  • GitHub Check: integration-test (ubuntu-latest, pypy3.10)
  • GitHub Check: integration-test (ubuntu-latest, 3.13)
  • GitHub Check: integration-test (ubuntu-latest, 3.9)
  • GitHub Check: integration-test (ubuntu-latest, pypy3.9)
  • GitHub Check: integration-test (ubuntu-latest, 3.12)
  • GitHub Check: integration-test (ubuntu-latest, 3.11)
  • GitHub Check: integration-test (ubuntu-latest, 3.10)
🔇 Additional comments (10)
pyproject.toml (1)

61-61: LGTM!

The addition of pytest-asyncio>=1.2.0 as a dev dependency properly supports the new async test coverage introduced in this PR.

pytest.ini (1)

13-14: LGTM!

These pytest-asyncio configuration options are appropriate for the new async test suite. The auto mode simplifies test marking, and function scope ensures proper event loop isolation per test.

centraldogma/_sync/base_client.py (1)

62-63: LGTM!

The __enter__ method correctly implements the synchronous context manager protocol, enabling with BaseClient(...) as client: usage. This pairs properly with the existing __exit__ method.

utils/run-unasync.py (1)

82-82: LGTM!

The __aenter__ → __enter__ mapping is correctly added to support async-to-sync transformation for context manager support, consistent with the existing __aexit__ → __exit__ mapping.

centraldogma/_async/base_client.py (2)

62-63: LGTM!

The __aenter__ method correctly implements the async context manager protocol, enabling async with BaseClient(...) as client: usage. This pairs properly with the existing __aexit__ method.

81-81: Critical fix: properly awaits AsyncRetrying coroutine.

This is the key bug fix mentioned in the PR description. AsyncRetrying returns a coroutine that must be awaited - without this await, the method would return a coroutine object instead of executing the retry logic and returning the actual result, causing a runtime error.

tests/test_async_base_client.py (4)

29-46: LGTM!

The fixtures are well-designed, using async context managers to properly test the new __aenter__ implementation. The client_with_configs fixture provides good coverage for various configuration options.

48-67: LGTM!

This test comprehensively validates _set_request_headers behavior across different HTTP methods, including the special application/json-patch+json content-type for PATCH requests. The assertions verify both the presence of expected headers/params and the absence of internal configuration keys.

138-162: LGTM!

Excellent retry coverage! The tests validate two retry scenarios:

  • test_get_with_retry_by_response: Retries on failed status codes (503 → 404 → 200)
  • test_get_with_retry_by_client: Retries on connection errors (ConnectError → ConnectError → NetworkError → 200)

Both tests correctly verify the expected call counts, confirming the retry logic works as intended.

164-205: LGTM!

The PATCH and POST tests provide solid coverage for:

  • Correct content-type headers (application/json-patch+json for PATCH, application/json for POST)
  • JSON payload echo verification
  • Authorization error handling (401 → UnauthorizedException)

The tests properly assert both request headers and payload integrity.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@hexoul hexoul requested review from corona10, ikhoon and minwoox December 7, 2025 03:17
ikhoon
ikhoon approved these changes Dec 10, 2025
View reviewed changes
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. 👍👍

minwoox
minwoox approved these changes Dec 10, 2025
View reviewed changes
Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@hexoul hexoul merged commit fdd3efa into line:main Dec 10, 2025
26 checks passed
@hexoul hexoul deleted the test/add-async-base branch January 11, 2026 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ikhoon ikhoon ikhoon approved these changes

@minwoox minwoox minwoox approved these changes

@corona10 corona10 Awaiting requested review from corona10

Assignees

@hexoul hexoul

Labels

defect improvement

Projects

None yet

Milestone

0.7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@hexoul @ikhoon @minwoox